Skip to content

Conversation

@findleyr
Copy link
Contributor

This CL implements two changes that must be logically connected.

The first is to provide a mechanism for disabling stream replay. Previously, our documentation stated that if
StreamableServerTransport.EventStore was nil, a default in-memory event store would be used. In that case, there would be no way to completely disable stream replay, and as described in #576, there are many cases where replay is undesirable.

But of course changing the behavior of the transport zero value implicitly affects its default behavior, and so this CL also implements the proposal #580: stream replay is disabled by default.

Implementing this change required a significant refactoring, as previously we were relying on the event store for serialized message delivery from the JSON-RPC layer to the MCP layer: the connection would write to the event store, and independently the stream (be it an incoming POST or replay GET) would iterate and serve messages in the stream.

In order to achieve the goals of this CL, it was necessary to decouple storage from delivery. The 'stream' abstraction now tracks a delivery callback that writes straight to the HTTP response. It would have been convenient to store the ongoing http.ResponseWriter directly in the stream (this is how typescript does it), but due to the design of our EventStore API, only the HTTP handler knows the next event index, so a 'deliver' abstraction was an unfortunate requirement (suggestions for how to further simplify this are welcome).

More simplification is possible: in particular, as a result of this refactoring it should be entirely possible to clean up streams once we've received all responses. Any replay would only need access to the EventStore, if at all. This is left to a follow-up CL, to limit this already significant change.

Furthermore, a nice consequence of this refactoring is that, when not using event storage, servers can get synchronous feedback that message delivery failed, which should avoid unnecessary work. We can additionally cancel ongoing requests on early client termination, but that is also left to a follow-up CL.

Throughout, the terminology 'standalone SSE stream' replaced 'hanging GET' when referring to the non-replay GET request issued by the client. This is consistent with other SDKs.

Fixes #576
Updates #580

neild
neild previously approved these changes Oct 15, 2025
neild
neild previously approved these changes Oct 16, 2025
This CL implements two changes that must be logically connected.

The first is to provide a mechanism for disabling stream replay.
Previously, our documentation stated that if
StreamableServerTransport.EventStore was nil, a default in-memory event
store would be used. In that case, there would be no way to completely
disable stream replay, and as described in modelcontextprotocol#576, there are many cases
where replay is undesirable.

But of course changing the behavior of the transport zero value
implicitly affects its default behavior, and so this CL also implements
the proposal modelcontextprotocol#580: stream replay is disabled by default.

Implementing this change required a significant refactoring, as
previously we were relying on the event store for serialized message
delivery from the JSON-RPC layer to the MCP layer: the connection would
write to the event store, and independently the stream (be it an
incoming POST or replay GET) would iterate and serve messages in the
stream.

In order to achieve the goals of this CL, it was necessary to decouple
storage from delivery. The 'stream' abstraction now tracks a delivery
callback that writes straight to the HTTP response. It would have been
convenient to store the ongoing http.ResponseWriter directly in the
stream (this is how typescript does it), but due to the design of our
EventStore API, only the HTTP handler knows the next event index, so a
'deliver' abstraction was an unfortunate requirement (suggestions for
how to further simplify this are welcome).

More simplification is possible: in particular, as a result of this
refactoring it should be entirely possible to clean up streams once
we've received all responses. Any replay would only need access to the
EventStore, if at all. This is left to a follow-up CL, to limit this
already significant change.

Furthermore, a nice consequence of this refactoring is that, when not
using event storage, servers can get synchronous feedback that message
delivery failed, which should avoid unnecessary work. We can
additionally cancel ongoing requests on early client termination, but
that is also left to a follow-up CL.

Throughout, the terminology 'standalone SSE stream' replaced 'hanging
GET' when referring to the non-replay GET request issued by the client.
This is consistent with other SDKs.

Fixes modelcontextprotocol#576
Updates modelcontextprotocol#580
@findleyr
Copy link
Contributor Author

@neild could you please re-approve when you have a chance? I had to rebase and resolve merge conflicts (I also squashed since I had to force-push anyway).

No changes other than resolving the conflict.

@jba jba merged commit 104d39c into modelcontextprotocol:main Oct 16, 2025
5 checks passed
findleyr added a commit to findleyr/go-sdk that referenced this pull request Oct 16, 2025
findleyr added a commit that referenced this pull request Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Streamable HTTP: no way to disable support for event replay

3 participants